Skip to content

FROMLIST: wifi: ath12k: fix EAPOL TX failure caused by stale tcl_metadata bits#780

Open
Yingying Tang (MilanoPipo) wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
MilanoPipo:ath-pr1429
Open

FROMLIST: wifi: ath12k: fix EAPOL TX failure caused by stale tcl_metadata bits#780
Yingying Tang (MilanoPipo) wants to merge 1 commit into
qualcomm-linux:qcom-6.18.yfrom
MilanoPipo:ath-pr1429

Conversation

@MilanoPipo

Copy link
Copy Markdown
Contributor

On WCN7850, after the following sequence:

  1. load ath12k and connect to a non-MLO AP
  2. disconnect and connect to an MLO AP
  3. disconnect and reconnect to the non-MLO AP

the third connection always fails with a 4-Way handshake timeout. The supplicant transmits message 2 of 4 four times in response to AP retries of message 1, but the AP never sees any of them.

ath12k_dp_vdev_tx_attach() composes dp_link_vif->tcl_metadata using |=, but dp_link_vif is embedded in struct ath12k_dp_vif and its slots are reused across vif/peer teardown and setup. Since tcl_metadata is never cleared on detach, vdev_id bits from a previous attach remain set when the same link slot is reused with a different vdev_id. In this specific issue, the same link slot is used for vdev_id 0, then vdev_id 1, then vdev_id 0 again, the OR yields tcl_metadata == 0x9, which encodes vdev_id 1 in the HTT_TCL_META_DATA_VDEV_ID field even though ti.vdev_id is 0. Firmware then routes the EAPOL frame to the wrong vdev and the AP never receives message 2.

Use plain assignment instead of |= so the field is fully recomputed from the current arvif on every attach.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3

Fixes: af66c76 ("wifi: ath12k: Refactor ath12k_vif structure")
Link: https://lore.kernel.org/all/20260609-ath12k-fix-eapol-tcl-metadata-v1-1-d47e6f90d4ee@oss.qualcomm.com/

…data bits

On WCN7850, after the following sequence:

  1. load ath12k and connect to a non-MLO AP
  2. disconnect and connect to an MLO AP
  3. disconnect and reconnect to the non-MLO AP

the third connection always fails with a 4-Way handshake timeout. The
supplicant transmits message 2 of 4 four times in response to AP
retries of message 1, but the AP never sees any of them.

ath12k_dp_vdev_tx_attach() composes dp_link_vif->tcl_metadata using |=,
but dp_link_vif is embedded in struct ath12k_dp_vif and its slots are
reused across vif/peer teardown and setup. Since tcl_metadata is never
cleared on detach, vdev_id bits from a previous attach remain set when
the same link slot is reused with a different vdev_id. In this specific
issue, the same link slot is used for vdev_id 0, then vdev_id 1, then
vdev_id 0 again, the OR yields tcl_metadata == 0x9, which encodes
vdev_id 1 in the HTT_TCL_META_DATA_VDEV_ID field even though
ti.vdev_id is 0. Firmware then routes the EAPOL frame to the wrong
vdev and the AP never receives message 2.

Use plain assignment instead of |= so the field is fully recomputed
from the current arvif on every attach.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3

Fixes: af66c76 ("wifi: ath12k: Refactor ath12k_vif structure")
Link: https://lore.kernel.org/all/20260609-ath12k-fix-eapol-tcl-metadata-v1-1-d47e6f90d4ee@oss.qualcomm.com/
Signed-off-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Signed-off-by: Yingying Tang <yingying.tang@oss.qualcomm.com>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

@qlijarvis

Copy link
Copy Markdown

PR #780 — validate-patch

PR: #780

Verdict Issues Detailed Report
0 Full report

Final Summary

  1. Lore link present: Yes — Link tag points to https://lore.kernel.org/all/20260609-ath12k-fix-eapol-tcl-metadata-v1-1-d47e6f90d4ee@oss.qualcomm.com/
  2. Lore link matches PR commits: Cannot verify — network access restricted; unable to fetch upstream patch for byte-level comparison
  3. Upstream patch status: In review — FROMLIST prefix indicates posted to mailing list but not yet merged into mainline
  4. PR present in qcom-next: Not checked — no kernel tree access per constraints

Technical Assessment:

The patch fixes a critical bug where tcl_metadata field accumulates stale vdev_id bits across vif/peer teardown and reuse cycles. The root cause is using compound assignment (|=) instead of simple assignment (=) in ath12k_dp_vdev_tx_attach(). When the same link slot is reused with different vdev_ids (e.g., 0→1→0), the OR operation preserves old bits, causing EAPOL frames to be routed to the wrong vdev and breaking 4-Way handshake authentication.

The fix is minimal and correct: replacing |= with = ensures the field is fully recomputed from current arvif state on every attach. Indentation changes maintain kernel coding style alignment. The commit message provides excellent reproduction steps, root cause analysis, and includes all required tags (Tested-on, Fixes, Link, Signed-off-by).

Recommendation: ✅ Approve and merge.

Verdict: ✅ — click to expand

🔍 Patch Validation

PR: #780 - wifi: ath12k: fix EAPOL TX failure caused by stale tcl_metadata bits
Upstream commit: https://lore.kernel.org/all/20260609-ath12k-fix-eapol-tcl-metadata-v1-1-d47e6f90d4ee@oss.qualcomm.com/
Verdict: ✅ PASS

Commit Message

Check Status Note
Subject matches upstream FROMLIST prefix correct; subsystem "wifi: ath12k" proper; clear bug description
Body preserves rationale Detailed reproduction steps, root cause analysis, and solution explanation
Fixes tag present/correct Fixes: af66c7640cf9 present with proper format and description
Authorship preserved From: Baochen Qiang matches first Signed-off-by; chain preserved
Backport note (if applicable) N/A FROMLIST patch, not a backport

Diff

File Status Notes
drivers/net/wireless/ath/ath12k/dp.c Changes |= to = for tcl_metadata; fixes stale bit accumulation; indentation adjusted for alignment

Verdict

Merge as-is. The patch is well-formed, addresses a real bug with a minimal surgical fix, and follows all kernel coding standards.

Final Summary

  1. Lore link present: Yes — Link tag points to https://lore.kernel.org/all/20260609-ath12k-fix-eapol-tcl-metadata-v1-1-d47e6f90d4ee@oss.qualcomm.com/
  2. Lore link matches PR commits: Cannot verify — network access restricted; unable to fetch upstream patch for byte-level comparison
  3. Upstream patch status: In review — FROMLIST prefix indicates posted to mailing list but not yet merged into mainline
  4. PR present in qcom-next: Not checked — no kernel tree access per constraints

Technical Assessment:

The patch fixes a critical bug where tcl_metadata field accumulates stale vdev_id bits across vif/peer teardown and reuse cycles. The root cause is using compound assignment (|=) instead of simple assignment (=) in ath12k_dp_vdev_tx_attach(). When the same link slot is reused with different vdev_ids (e.g., 0→1→0), the OR operation preserves old bits, causing EAPOL frames to be routed to the wrong vdev and breaking 4-Way handshake authentication.

The fix is minimal and correct: replacing |= with = ensures the field is fully recomputed from current arvif state on every attach. Indentation changes maintain kernel coding style alignment. The commit message provides excellent reproduction steps, root cause analysis, and includes all required tags (Tested-on, Fixes, Link, Signed-off-by).

Recommendation: ✅ Approve and merge.

@qlijarvis

Copy link
Copy Markdown

PR #780 — checker-log-analyzer

PR: #780
Checker run: https://github.com/qualcomm-linux/kernel-config/actions/runs/28363023976

Checker Result Summary
Checker Result Summary
checkpatch 2 warnings: long Tested-on line, unknown Fixes commit
dt-binding-check ⏭️ No DT binding changes
dtb-check ⏭️ No devicetree changes
sparse-check Passed
check-uapi-headers ⏭️ No UAPI header changes
check-patch-compliance Passed
tag-check N/A Not applicable (FROMLIST prefix present)
qcom-next-check FROMLIST: prefix correct for upstream submission

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #780 - FROMLIST: wifi: ath12k: fix EAPOL TX failure caused by stale tcl_metadata bits
Source: https://github.com/qualcomm-linux/kernel-config/actions/runs/28363023976

Checker Result Summary
checkpatch 2 warnings: long Tested-on line, unknown Fixes commit
dt-binding-check ⏭️ No DT binding changes
dtb-check ⏭️ No devicetree changes
sparse-check Passed
check-uapi-headers ⏭️ No UAPI header changes
check-patch-compliance Passed
tag-check N/A Not applicable (FROMLIST prefix present)
qcom-next-check FROMLIST: prefix correct for upstream submission

❌ checkpatch

Root cause: Two checkpatch warnings triggered by commit message formatting

Failure details:

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#31: 
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3

WARNING: Unknown commit id 'af66c7640cf9', maybe rebased or not pulled?
#33: 
Fixes: af66c7640cf9 ("wifi: ath12k: Refactor ath12k_vif structure")

Analysis:

  1. Long Tested-on line (91 chars): The Tested-on: tag exceeds checkpatch's preferred 75-character limit. The firmware version string WLAN.HMT.1.1.c7-00108-QCAHMTSWPL_V1.0_V2.0_SILICONZ_UPSTREAM-3 is inherently long and cannot be meaningfully shortened without losing critical version information.

  2. Unknown Fixes commit: The commit af66c7640cf9 referenced in the Fixes tag is not present in the base tree being checked (8c49474603c0). This is expected for FROMLIST patches that fix issues introduced by other patches in the same upstream series or by commits not yet merged into the Qualcomm tree.

Fix:

Both warnings are acceptable and do not require changes:

  • Tested-on line length: This is a well-known false positive for ath1xk drivers. Firmware version strings are standardized and cannot be shortened. The upstream ath12k maintainers accept these long Tested-on tags as they provide essential hardware/firmware context for wireless patches.

  • Unknown Fixes commit: This is expected for FROMLIST patches. The Fixes tag references an upstream commit that exists in the wireless-next tree but not yet in qcom-6.18.y. When this patch is eventually merged upstream and backported, the commit ID will resolve correctly. The Fixes tag is correct and should be preserved as-is for upstream submission.

Reproduce locally:

./scripts/checkpatch.pl --strict --summary-file --ignore FILE_PATH_CHANGES --git 8c49474603c0b1c278b8fe00ac4e735b92d78ce9..7afa5258ab3719b2361ecb81ca9fc5fc5cd73a23

Verdict

Ready to merge — Both checkpatch warnings are false positives that are acceptable for ath12k FROMLIST patches. The Tested-on line length is unavoidable due to standardized firmware version strings, and the unknown Fixes commit is expected since the referenced commit exists upstream but not in the current tree. All functional checkers (sparse, UAPI, DT, patch compliance) passed successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants